Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(adm): Use Firefox's full_keyword algorithm #214

Merged
merged 3 commits into from
Nov 10, 2021

Conversation

mythmon
Copy link
Contributor

@mythmon mythmon commented Nov 10, 2021

Notably, this algorithm never suggests more than the rest of the current word.

Fixes #213

Notably, this algorithm never suggests more than the rest of the current word.

Fixes #213
@mythmon mythmon requested a review from a team as a code owner November 10, 2021 19:25
new_suggestions.insert(keyword.clone(), merino_suggestion.clone());
let full_keyword = Self::get_full_keyword(keyword, &adm_suggestion.keywords);

if keyword == "amaz" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debugging leftover?

Copy link
Contributor

@ncloudioj ncloudioj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+ w/ comments.

/// the available keywords.
///
/// 1. Find the first keyword phrase that has more words than the query. Use
/// its first `queryWords.length` words as the full keyword. e.g., if the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the doc to reflect the variables used in the function? queryWords and result.keywords are a little confusing here.

@mythmon mythmon enabled auto-merge (squash) November 10, 2021 22:02
@mythmon mythmon merged commit 009bd1e into main Nov 10, 2021
@mythmon mythmon deleted the full-keyword-is-only-next-word branch November 10, 2021 22:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merino Sponsored/Non-Sponsored results have extra keywords suggested
2 participants